Skip to content

feat(cockpit): make Run discoverable from Code - #934

Merged
blove merged 18 commits into
mainfrom
blove/cockpit-code-discoverability-1ce4a0
Sep 1, 2026
Merged

feat(cockpit): make Run discoverable from Code#934
blove merged 18 commits into
mainfrom
blove/cockpit-code-discoverability-1ce4a0

Conversation

@blove

@blove blove commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Why

On a capability page in Code view, nothing tells you the example can be run. The mode rail exists and works, but it doesn't read as a switch.

Three failures, measured on the live page at 1440×900:

  1. The rail reads as chrome. The four mode buttons sat at the same visual weight as the Activity/Settings utilities below them, nothing grouped them, and inactive items used --ds-text-muted — the same token as disabled text, so they read as unavailable.
  2. Code view has no exit affordance. ~1250px of the viewport is tree + tabs + source; the only route back to Run is a 48px target ~1200px away.
  3. Runtime controls sat ~1000px below the fold. The Runtime heading rendered at y=1860 in a 900px viewport, unreachable without scrolling past ~30 nav links.

Amplifier: activeMode was persisted under one global localStorage key, not scoped per capability. One exploratory click into Code made Code the landing view for every capability on every future visit — verified live on production.

What changed

1 Mode is no longer sticky — every capability opens in Run; ?mode= deep links still work
2 Rail reads as a switch — resting contrast off the disabled token, a visible rule separating modes from utilities, a VIEW group label
3 Generic status?: { kind, label } slot on ControlPlaneRailItem
4 Run carries a live runtime phase dot — 8 phases → 4 buckets, state in the accessible name
5 Activity re-scoped to unread problems, so the two dots make distinct claims

Run now says what the runtime is doing; Activity says there are problems you haven't read. That's also strictly better on one case: a runtime that fails and self-recovers previously left no trace once the phase cleared — now the unread error survives until someone looks.

Scope note

This deliberately does not address failure 2. Nothing lands in the 1250px where your eye is while reading code. The rail becomes legible; it does not become close. This was chosen as a first step, to evaluate before deciding whether an in-content affordance is still needed.

Verification

cockpit 433/433, ui-react 33/33, nx lint ui-react 0 errors.

Browser-verified against a fresh build at 1440×900, both themes:

  • Stale activeMode: "Code" in storage → lands on Run
  • ?mode=code → lands on Code, search stripped to ""
  • Code → click another capability → lands on Run (the key={canonicalPath} fix)
  • Rail inactive rgb(200,200,200) dark / rgb(70,70,70) light; separator visible in both
  • Run dot position: absolute, 7×7, rgb(76,195,138) dark / rgb(26,122,64) light
  • Run has no stray tooltip; status lives in a visually-hidden span
  • No console or hydration errors

Not verified by eye (covered by tests; live repro needs a deliberately broken runtime): the Activity dot on a real failure, and the working/error dot colours in situ.

Notable during review

  • The separator initially shipped invisible. It used --ds-borderrgb(45,45,45) on a rgb(44,44,44) rail in dark, a one-value difference — while every test passed. Fixed to --ds-border-strong, with a comment recording why.
  • A test guarding the route key was circular — deleting the key from page.tsx left the suite fully green. Replaced with a route-module test that actually fails.
  • The plan I wrote had two errors, both caught by reading the code rather than trusting the prose: activityReducer prepends (the specified selector was inverted, and all three specified tests passed either way), and runtimeNeedsAttention had no remaining caller (now deleted). Spec and plan corrected.

Design: docs/superpowers/specs/2026-08-31-cockpit-run-discoverability-design.md
Plan: docs/superpowers/plans/2026-08-31-cockpit-run-discoverability.md

🤖 Generated with Claude Code

@vercel

vercel Bot commented Sep 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
threadplane Ready Ready Preview Sep 1, 2026 9:21pm UTC

Request Review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: this PR received an intelligent (AI) code review. See the review comments on this PR.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

blove and others added 18 commits September 1, 2026 14:17
Records the brainstormed design: mode stops being sticky, the mode rail
becomes legible as a switch, Run carries a runtime phase dot, and the
Activity dot is re-scoped to unseen problems so the two dots make
distinct claims.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six TDD tasks: drop mode stickiness, make the rail legible as a switch,
add a status-dot slot to the ui-react rail primitive, wire the runtime
phase onto Run, re-scope the Activity indicator to unseen problems, and
verify in a browser.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ns in Run

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ten mode tests

Address code-review findings on 940e21d8:
- Restore the only coverage of the docs-surface setExpanded branch, lost
  when the mode-persistence tests were rewritten (mutation-verified by
  flipping `surface === 'docs'` to `false`).
- Key CockpitShell on canonicalPath in the [...slug] route so client-side
  navigation between capabilities always remounts fresh state, instead of
  relying on React Router reconciliation behavior that may or may not
  preserve activeMode across a params-only navigation. Add a component
  test proving a capability switch lands on Run even after switching to
  Code, mutation-verified by reverting to a shared key.
- Give the invalid-mode-query test discriminating power by asserting the
  query param is stripped, not just that the default mode is shown.
- Fix a prettier violation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
apps/cockpit has no lint target (only ui-react does), and nx test
swallows the vitest reporter output in this worktree, so real counts
need vitest run --root directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Shell key

The component-level test added in 58f55680 hard-coded key={...} in its own
JSX and never imported the route module, so it only proved React's
remount-on-key-change semantics rather than guarding the actual production
wiring. Deleting key={canonicalPath} from page.tsx left the full cockpit
suite green.

Add page.spec.tsx, which imports the [...slug] route module directly and
asserts the returned element's key equals the canonical path for a slug,
and that two different capabilities get two different keys. next/navigation
and the content-bundle loader are mocked so the route can be exercised in
isolation.

page.tsx also needed an explicit `import React from 'react'` (matching the
sibling apps/cockpit/src/app/page.tsx) — without it, evaluating the module
under Vitest's jsx transform threw ReferenceError: React is not defined
before the route logic ever ran.

Mutation-verified: with key={canonicalPath} removed, both new tests fail
(element.key is null); restored via `git checkout --` and confirmed
`git diff --exit-code` was clean before reapplying the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Group the four mode buttons under a "View" caption, separate them
from the Activity/Settings utilities with a rule line, and lift
inactive rail items off --ds-text-muted (the disabled-text token)
onto --ds-text-secondary so the rail reads as a switch, not chrome.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two review fixes to 3d43dd54:

- The utilities-group separator used --ds-border, which is a one-value
  difference from --ds-surface-tinted (the rail background) in dark
  mode (rgb(45,45,45) on rgb(44,44,44)) -- effectively invisible in
  the cockpit's default theme. Switch to --ds-border-strong, the token
  the pane divider already uses for this same collision.

- The "View" caption on the mode group was aria-hidden, on the theory
  that the nav's own aria-label covered it. It doesn't: Activity and
  Settings render inside the same <nav>, so a screen-reader user heard
  one flat "Cockpit modes" sequence with no group boundary -- sighted
  users got the new two-group structure, non-sighted users didn't.
  Replace aria-hidden with role="group" + aria-labelledby on the
  primary group div, which announces "View" as a group label without
  adding a second landmark.

Also bumped the caption to --ds-text-secondary/10px to match
[data-control-plane-rail-label], and made the spec's cockpit.css path
resolution cwd-independent via import.meta.url instead of a
process.cwd() suffix check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
status and statusLabel were independent optional props, so either
could be set without the other: a status with no accessible label
(colour-only, WCAG 1.4.1) or a label with no dot. Combine them into
a single ControlPlaneRailItemStatus object so both invalid states
are unrepresentable. No consumers of either prop existed yet, so
this is a clean API tightening, not a migration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 3 tightened status/statusLabel into one { kind, label } object;
update Task 4's mapping, wiring and mutation check to match, and record
the flex-gap and tooltip-placement hazards it must handle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A labelled rail item with a status no longer renders the icon-rail
tooltip, whose positioning is authored for the 56px rail and lands over
the context pane. The status stays in the accessible name via aria-label
and in a visually hidden span. The status dot's ring now tracks the rail
item background so hover does not show a halo, and the dot keeps an
explicit forced-colors treatment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Run's rail dot now reports what the runtime is doing right now, so the
Activity dot no longer duplicates it. Activity means "there are problems
in the log you have not read": errors only, counted from a seen marker
that the panel clears on open. An error that self-recovers keeps its
trace until someone looks.

Also makes runtimeNeedsAttention an exhaustive switch so a new
RuntimePhase fails to compile there as it already does in
runtimeRailStatus, and decouples the Run rail assertions from the now
phase-dependent accessible name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
activityReducer prepends, so the log is newest-first: the unseen window
runs from index 0, not from seenCount. The plan asserted the opposite and
its three specified tests passed under both readings, so the inverted
selector would have shipped green. Record the direction and the
count-marker saturation limit at the 50-event cap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both the spec and the plan claimed the helper still had a caller --
the spec said it drives the Run dot's error bucket, the plan said
use-runtime-controller.ts uses it. Neither is true: runtimeRailStatus
covers the Run dot, and removing the Activity dot's call left zero
production callers. Record that it is deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r path

Re-scoping the Activity dot removed the last caller of
runtimeNeedsAttention; the Run dot goes through runtimeRailStatus. An
exported, tested, uncalled predicate is what gets miswired back later, so
delete it and its test table rather than hardening it.

Adds the missing clear-path coverage: resetting the seen marker on clear
is load-bearing, and dropping it leaves the marker stranded over an empty
log, swallowing the next N problems for the rest of the page visit.

Also drops a provably dead inner clamp in countUnseenProblems (slice
already clamps a too-large end index) while keeping the outer one, which
is not defensive -- a negative end index counts back from the tail and
reports already-seen errors as unseen. The test for it now proves that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All six tasks verified live in both themes against a fresh build.
Records two false alarms caused by testing under a live dev server
after git-level file swaps, and the procedure that avoids them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@blove
blove force-pushed the blove/cockpit-code-discoverability-1ce4a0 branch from 10e4938 to e87eb94 Compare September 1, 2026 21:18
@blove
blove enabled auto-merge (squash) September 1, 2026 21:18
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

@blove
blove merged commit 44388f2 into main Sep 1, 2026
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant